Repository navigation
[OMEGA-490] Remove legacy antispam filter, fix dropped token limit notifications - #383
paul-v-snet wants to merge 3 commits into
Conversation
|
|
| (progn (change-state! &lastsend $msg) | ||
| (let $safemsg (string-replace $msg "\n" "\\n") | ||
| (let $temp (cut) (commChannelSend $safemsg)))) _)) | ||
| (progn (change-state! &lastsend $msg) |
There was a problem hiding this comment.
Do we still need &lastsend here now that the deduplication check has been removed? It looks like the value is no longer read.
There was a problem hiding this comment.
Right, now it's not used anywhere, but I just decided to keep it in case it will be useful in the future.
So we can keep it or remove it, that actually won't affect anything, or I can just add a comment with a clarification.
What do you prefer?
There was a problem hiding this comment.
I’d prefer to remove it for now, since nothing reads it anymore.
|
@paul-v-snet, tested 5910f6b over Notices per phase
Patch: one notice per user messageA new message ( --- a/providers/lib_llm_ext.py
+++ b/providers/lib_llm_ext.py
@@ -4,3 +4,3 @@ from typing import Optional, Tuple, Dict, Any
from config import config_get_by_key
-from src.helper import quote_arg
+from src.helper import quote_arg, split_command_blocks
from src.logger import get_logger
@@ -69,2 +69,26 @@ def _llm_empty_response_command() -> str:
+_limit_notice_pending = False
+
+def _sends_to_user(raw: str) -> bool:
+ text = raw.replace("_quote_", '"').replace("_newline_", "\n")
+ for block in split_command_blocks(text):
+ command = block.lstrip().lstrip("(").split(maxsplit=1)
+ if command and command[0].rstrip(")") == "send":
+ return True
+ return False
+
+def _reply_or_limit_notice(content: str, raw: str, cut_by_limit: bool) -> str:
+ global _limit_notice_pending
+ _, usermsg = _split_system_user(content)
+ if "HUMAN-MSG:" in usermsg:
+ _limit_notice_pending = True
+ if raw:
+ if _sends_to_user(raw):
+ _limit_notice_pending = False
+ return raw
+ if cut_by_limit and _limit_notice_pending:
+ _limit_notice_pending = False
+ return _llm_empty_response_command()
+ return raw
+
def _split_system_user(content: str) -> Tuple[str, str]:
@@ -190,4 +214,3 @@ class AIProvider(AbstractAIProvider):
logger.warning("LLM returned an empty response")
- if finish_reason == "length":
- raw = _llm_empty_response_command()
+ raw = _reply_or_limit_notice(content, raw, finish_reason == "length")
resp = self._clean_text(raw)
--- a/providers/asione.py
+++ b/providers/asione.py
@@ -77,4 +77,3 @@ class ASIOneProviderImpl(llm.AIProvider):
logger.warning("LLM returned an empty response")
- if finish_reason == "length":
- raw = llm._llm_empty_response_command()
+ raw = llm._reply_or_limit_notice(content, raw, finish_reason == "length")
resp = self._clean_text(raw)
--- a/providers/openai.py
+++ b/providers/openai.py
@@ -64,4 +64,3 @@ class OpenAIProviderImpl(llm.AIProvider):
logger.warning("LLM returned an empty response")
- if incomplete_reason == "max_output_tokens":
- raw = llm._llm_empty_response_command()
+ raw = llm._reply_or_limit_notice(content, raw, incomplete_reason == "max_output_tokens")
return self._clean_text(raw)
--- a/Autotests/unit/test_llm_budget.py
+++ b/Autotests/unit/test_llm_budget.py
@@ -57,3 +57,9 @@ asione = _MODULES["asione"]
-PROMPT = "You are an agent. :-:-:-: Write an empty line to /tmp/paths.txt"
+PROMPT = "You are an agent. :-:-:-: (HUMAN-MSG: Write an empty line to /tmp/paths.txt)"
+IDLE_PROMPT = "You are an agent. :-:-:-: "
+
+
+@pytest.fixture(autouse=True)
+def no_pending_limit_notice():
+ llm._limit_notice_pending = False
@@ -158,2 +164,41 @@ def test_asione_empty_reply_out_of_budget_is_explained():
+@pytest.mark.parametrize("make, response", [
+ (make_openrouter, lambda: chat_response("", "length")),
+ (make_asione, lambda: chat_response("", "length")),
+ (make_openai, lambda: responses_response("", "incomplete", "max_output_tokens")),
+])
+@pytest.mark.parametrize("prompt", [IDLE_PROMPT, "You are an agent. :-:-:-: DO NOT RE-SEND OR SPAM!"])
+def test_empty_reply_out_of_budget_without_a_user_message_sends_nothing(make, response, prompt):
+ assert make(FakeCreate(response())).chat(prompt) == ""
+
+
+def test_limit_notice_comes_once_after_a_step_without_an_answer():
+ provider = make_openrouter(FakeCreate(
+ chat_response('(query "hash maps")', "length"),
+ chat_response("", "length"),
+ chat_response("", "length"),
+ ))
+ assert provider.chat(PROMPT) == '(query "hash maps")'
+ assert sent_text(provider.chat(IDLE_PROMPT)) == llm.LLM_EMPTY_RESPONSE_MESSAGE
+ assert provider.chat(IDLE_PROMPT) == ""
+
+
+@pytest.mark.parametrize("answer", ['(send "hi")', 'send "hi"', 'Sure.\nsend "hi"', '(query "x")\n(send "hi")'])
+def test_limit_notice_is_skipped_once_the_user_got_an_answer(answer):
+ provider = make_openrouter(FakeCreate(
+ chat_response(answer, "stop"),
+ chat_response("", "length"),
+ ))
+ assert provider.chat(PROMPT) == answer
+ assert provider.chat(IDLE_PROMPT) == ""
+
+
+def test_every_user_message_cut_by_the_limit_gets_its_own_notice():
+ provider = make_openrouter(FakeCreate(*(chat_response("", "length") for _ in range(4))))
+ assert sent_text(provider.chat(PROMPT)) == llm.LLM_EMPTY_RESPONSE_MESSAGE
+ assert provider.chat(IDLE_PROMPT) == ""
+ assert sent_text(provider.chat(PROMPT)) == llm.LLM_EMPTY_RESPONSE_MESSAGE
+ assert provider.chat(IDLE_PROMPT) == ""
+
+
@pytest.mark.parametrize("effort, max_tokens, want_budget, want_enabled", [With the patch:
Repeated replies now reach the userWith the default config, one answer was followed by "No new user input. Standing by." twice in a row, and one request got A retry with the same text is ignoredloop.metta:85 treats a message equal to the previous one as no new input, so the same text sent twice gets no answer the second time. It happens on v0.1.20 too and is outside the diff. After a token limit notice a user will likely retry with the same text, so this may deserve its own ticket. Leftover
|
Description
The issue was caused by a deprecated antispam mechanism that was removed from MeTTaClaw but, for some reason, still remained in Omega. After an internal discussion, we decided to remove it, which automatically fixes the issue.
How Has This Been Tested?
Checklist